fix: attribute instrument bootstrap() warnings to the user's call site (#202) - #208
Merged
Merged
Conversation
#202) The three warnings raised while an instrument bootstraps carried a literal `stacklevel`, which encodes a frame depth that is not constant. Measured on `main` before the change: opentelemetry_instrument.py -> bootstrappers/fastapi_bootstrapper.py:118 fastapi_bootstrapper.py -> bootstrappers/base.py:137 Both are lite-bootstrap's own source. They differ by one frame because `FastAPISwaggerInstrument.bootstrap()` is called straight from the loop in `BaseBootstrapper.bootstrap()` while `FastAPIOpenTelemetryInstrument.bootstrap()` calls `super().bootstrap()` first, so no single literal can be right for both, and any instrument that grows or loses a `super()` call shifts its own attribution silently. Route all three through `warn_at_caller`, which walks out to the first frame outside `lite_bootstrap`. On this path that frame is the user's `bootstrap()` call, which is the line they can act on: the warnings say a dependency is missing or that `swagger_path` is being ignored, and the fix for both is in the config they passed, not inside the instrument. The helper needed one addition, a `category` parameter, since the OpenTelemetry warnings are `InstrumentDependencyMissingWarning` rather than plain `UserWarning`. Two invariant tests pin both depths through the bootstrapper, one per shape; either alone would pass against a literal. The existing OpenTelemetry test covers the instrument called on its own, where the walk already stopped at the right frame, and its "what breaks it" paragraph is updated to name the helper. Out of scope, per the issue: the two warnings reached from `BaseBootstrapper.__init__` are correct today and keep their literal `stacklevel`, and `helpers/fastapi_helpers.py` warns from inside a request handler where no user frame exists at all. Closes #202
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #202.
The defect, re-measured
The issue's line numbers predate
c210381(#199), which moved both call sites. Re-measured oncurrent
main, the two shapes still diverge exactly as described:swagger_path differs from docs_urlbootstrappers/base.py:137—one_instrument.bootstrap()bootstrappers/fastapi_bootstrapper.py:118—super().bootstrap()Both are lite-bootstrap's own source, and they differ by one frame because
FastAPISwaggerInstrument.bootstrap()is called straight from the loop whileFastAPIOpenTelemetryInstrument.bootstrap()callssuper().bootstrap()first. No single literalstacklevelis right for both, and any instrument that grows or loses asuper()call shifts itsown attribution with nothing to notice.
The decision the issue left open
Resolved as the
bootstrap()call site. Both warnings are about the config the user passed — anoptional dependency that is missing, a
swagger_paththat is being ignored — so the line theywould edit in response is where they called
bootstrap(), not a frame inside an instrument. Thatalso makes the two shapes converge rather than stay one frame apart, which is the part of the
defect a corrected literal would have left standing.
So
warn_at_callerit is, as the issue anticipated, with one addition it did not: the helperhardcoded
UserWarning, and the two OpenTelemetry sites raiseInstrumentDependencyMissingWarning.A
categoryparameter defaulting toUserWarningcovers both, leaving the three existingconfig-path callers untouched.
Renaming
_CONSTRUCTION_MODULESto_INTERNAL_MODULESfollows from the widened contract: theframes being skipped are no longer only construction frames. It is module-private, so no alias is
owed.
Tests
Two invariant tests, one per depth, both through
FastAPIBootstrapper.bootstrap(). Either alonewould pass against a literal
stacklevel— it is the pair that pins the property. Both werewritten first and both failed on the two paths in the table above.
The existing
test_missing_exporter_warning_points_at_the_caller_of_bootstrapcovers theinstrument called on its own, where the walk already stopped at the right frame; its "what breaks
it" paragraph now names the helper rather than the literal it used to describe.
The filter-and-compare assertion those three tests share is now
conftest.warning_source_files().It compares whole lists rather than indexing
caught[0]: a warning raised from the wrong frame andone raised twice are different bugs, and the list form catches both.
Out of scope, per the issue
BaseBootstrapper's two construction-time warnings keep their literalstacklevel, as #202directed — they land on the user's line today.
helpers/fastapi_helpers.pyis likewise untouched:it warns while serving a request, with no user frame anywhere on the stack.
AGENTS.md's rule needed rewriting, as the issue required. Its old sentence — warnings outsideconfig construction keep their literal
stacklevel, targeting the bootstrapper's caller — is nowwrong on both halves. The replacement also inventories the three literal sites that survive and why
each is deliberate, including the detail that
_attach_teardown_once'sstacklevel=3is oneshallower than
_select_instruments'stacklevel=4because the subclass__init__calls itdirectly.
Verification
ruff format --check,ruff check --no-fixandty checkclean. 277 tests pass at 100% statementcoverage.